test(service-settings): add settings-route-ledger conformance guard - #17071
Merged
os-trump merged 2 commits intoSep 9, 2026
Conversation
registerSettingsRoutes is an exported, synchronous, top-level function (http, service, opts) => void that calls http.get/put/post directly and touches neither argument before a request arrives -- the same shape storage-routes.ts and datasource's admin-routes.ts export. This mirrors their conformance-test seam (a capturing mock IHttpServer) rather than i18n's plugin-lifecycle shape, since no lifecycle is needed to reach an already-exported registrar. Both directions verified red-then-restored before landing: deleting the PUT /api/settings/:namespace ledger row, and adding a mounted DELETE /api/settings/:namespace/:actionId route with no row, each fail the new test naming that exact route. The existing ledger was checked for drift first and found current (4 registrar calls, 4 ledger rows, 1:1). settings-route-ledger.ts's header comment previously said a per-package guard was deliberately omitted in favour of the dogfood live-mount-parity gate; updated now that this file exists alongside it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
Contributor
📓 Docs Drift Check
What this run could not see
Coarse fallback — 7 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): |
os-trump
marked this pull request as ready for review
September 9, 2026 10:38
os-trump
enabled auto-merge
September 9, 2026 10:38
os-trump
deleted the
claude/issue-17062-settings-route-ledger-conformance
branch
September 9, 2026 10:59
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #17062
What
Adds
packages/services/service-settings/src/settings-route-ledger.conformance.test.ts— the guard every OTHER*-route-ledger.tsin the tree already pairs itself with, missing here sinceSETTINGS_ROUTE_LEDGERlanded at #7526. Test-only; no behaviour change.How
settings-routes.tsregisters, and why this shaperegisterSettingsRoutes(settings-routes.ts) is an exported, synchronous, top-level function —(http, service, opts) => void— that callshttp.get/put/postdirectly and touches neitherservicenoroptsbefore a request arrives (both are only read inside handler closures). That is exactly the shapestorage-routes.tsand datasource'sadmin-routes.tsexport, and it's why their conformance tests already settled the right seam: drive the registrar against a capturing mockIHttpServerand read its recorded calls as the route set.It is not the i18n shape —
I18nServicePlugin's route registration is a private method reachable only by driving the plugin'sinit→start→kernel:readylifecycle.registerSettingsRoutesneeds no lifecycle to reach because it's already the exported seam; driving one it doesn't require would intercept nothing a direct call doesn't. It's also not a source-scan (cli/metadata/trigger-api's second limb) — that shape earns its keep when the mounting mechanism can't be driven behind a mock; here it's an ordinary function call. So this PR copies the storage/datasource shape, chosen by mechanism rather than by proximity.The direction that matters
packages/qa/dogfood/test/route-ledger-live-mount-parity.dogfood.test.ts(#7526) already asserts, on a real boot, that every ledgered settings row resolves — and (worth being precise about, since it changes the framing slightly from the issue) itsevery mounted route is ledgeredassertion does foldSETTINGS_ROUTE_LEDGERinto the union it checks, so a wholly new, unledgered settings route would in fact turn that dogfood test red too, today. But that coverage is (a) a cross-package aggregate — the failure names the bare route, not "service-settings' ledger drifted" — (b) gated on whatever plugins that one dogfood boot composes, and (c) lives inpackages/qa/dogfood, not in this package's ownpnpm test. It is not the fast, package-scoped, always-runs unit guard every sibling ledger has, and that is the gap this PR closes.settings-route-ledger.ts's own header comment previously claimed this second guard was deliberately omitted for exactly this reason; it's updated in this PR now that the guard exists.Ledger checked for staleness — found current, not stale
Read
settings-routes.tsend-to-end: it makes exactly four registration calls (GET /api/settings,GET /api/settings/:namespace,PUT /api/settings/:namespace,POST /api/settings/:namespace/:actionId), 1:1 withSETTINGS_ROUTE_LEDGER's four rows. Confirmed empirically too: the new test passes 6/6 on the unmutated tree. No data fix bundled in this PR; per the card's own fence, this would have been reported and sequenced separately had it been needed.Both directions shown red, then restored
Predicted before running, mutated on disk, restored under a shell
trap(git checkout HEAD -- PATH,PATHstanding for the file's real repo-relative path), restoration proven by blob equality and an emptygit diff HEAD. Both matched prediction exactly (full transcript + blob hashes in the linked issue comment / report JSON):PUT /api/settings/:namespaceledger row (route still mounted) → red, namingPUT /api/settings/:namespaceinevery mounted settings route has a ledger entry(live.size4 vsledgerKeys().size3).DELETE /api/settings/:namespace/:actionIdroute with no ledger row → red, naming that exact route in the same assertion (live.size5 vsledgerKeys().size4) — the direction the issue flags as the one a copied test can quietly omit.Changeset
None.
packages/services/service-settings/package.json'sfilesis["dist", "README.md", "CHANGELOG.md"]— a*.test.tsundersrc/is never published, so this diff ships nothing from any released package (skip-changeset, applied on this PR).Gates
pnpm --filter '@objectstack/service-settings^...' build— clean (dependency closure).pnpm --filter @objectstack/service-settings test— 578/578 passed (33 files), including the new conformance test.pnpm --filter @objectstack/service-settings typecheck— clean.pnpm check:route-envelope— clean (named explicitly per dispatch note; no route module touched, so N/A in substance).node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackderived 51 commands for this diff; all 51 ran (--ranreconciliation: 51 derived, 51 run, 0 NOT-MEASURED, 0 UNRUN). 49/51 exited 0. The other two are legitimatePREREQUISITE NOT MET(exit 3), not failures —check:dual-build-cjs-loadsandcheck:type-check-debt --re-measureboth refuse to run without a fullpnpm buildacross ~50 packages'dist/(only theservice-settingsdependency closure was built locally, per this lane's local-verification scope); CI'sLint & Repo Gatesjob builds the full closure first and will measure both for real.